ext/curl: speed up tests - #23228
Conversation
|
@NickSdot Could you take a look at this? |
NickSdot
left a comment
NickSdot
left a comment
There was a problem hiding this comment.
LGTM! Can confirm it's ~40% faster -- nice! Added one nit.
| try { | ||
| // Give the server time to start | ||
| sleep(1); | ||
| for ($i = 0; $i < 100; $i++) { |
There was a problem hiding this comment.
| for ($i = 0; $i < 100; $i++) { | |
| for ($i = 0; $i < 50; $i++) { |
Maybe 50 is enough? Would match the previous 1s; fsockopen already adds extra.
There was a problem hiding this comment.
Yes, 50 (or even 10 or 20) would work. 100 is just a number that's high but not infinite. In normal operation this loop will only iterate a couple of times, and 50 or 100 is never reached.
If you are worried about the time this test takes when the server fails to start, perhaps a better way is to get the server process status with proc_get_status and stop the test when the server is no longer running.
There was a problem hiding this comment.
Was just a nit; wouldn't over-complicate it personally. A server not starting would die out before we arrive here. It's rather for slower envs -- git history related to the server mentions some Travis CI (and other) edge cases. As you said, normally the max should not be hit -- since the 1s didn't flake 50ish is perhaps just fine.
|
|
||
| if (CURLOPT_INFILE == $curl_option) { | ||
| curl_setopt($ch, CURLOPT_UPLOAD, 1); | ||
| curl_setopt($ch, CURLOPT_HTTPHEADER, ['Expect:']); |
There was a problem hiding this comment.
Disable Expect: 100-continue to prevent libcurl's 1-second delay.
May I ask where the 1 second delay come from?
There was a problem hiding this comment.
Expect 100-continue is a flow control mechanism that is not apparently not supported by the PHP development server. Curl waits one second for a 100-continue response and then continues anyway.
https://everything.curl.dev/http/post/expect100.html
Unfortunately, lots of servers in the world do not properly support the Expect: header or do not handle it correctly, so curl only waits 1000 milliseconds for that first response before it continues anyway.
You can avoid the wait entirely by using -H Expect: to remove the header
There was a problem hiding this comment.
By waiting shorter for the server to be ready. In my setup, this takes it from 26 to 15 seconds for all curl tests. The first check for output on stderr takes approximately 10ms, so we wait 20ms to make sure its ready. The second check for the open port typically succeeds immediately, so move the sleep to after we have tried.
Disable Expect: 100-continue to prevent libcurl's 1-second delay Expect 100-continue is a flow control mechanism that is not apparently not supported by the PHP development server. Curl waits one second for a 100-continue response and then continues anyway. https://everything.curl.dev/http/post/expect100.html > Unfortunately, lots of servers in the world do not properly support the Expect: header or do not handle it correctly, so curl only waits 1000 milliseconds for that first response before it continues anyway. > You can avoid the wait entirely by using -H Expect: to remove the header
d8dd936 to
8fc637d
Compare
3 participants
By waiting shorter for the server to be ready. In my setup, this takes it from 26 to 15 seconds for all curl tests.
The first check for output on stderr takes approximately 10ms, so we wait 20ms to make sure its ready. The second check for the open port typically succeeds immediately, so move the sleep to after we have tried.
Disable Expect: 100-continue to prevent libcurl's 1-second delay.